fix: support persistent lifespans in asgi - #231
Conversation
|
I'm Bonk, and I've done a quick review of your PR. Makes ASGI entrypoint lifespan state persist across requests.
|
864cb45 to
5b8cb5e
Compare
hoodmane
left a comment
There was a problem hiding this comment.
Generally looks reasonable, though I'd set _start_future on the AsgiWorkerEntrypoint instance rather than the class and initialize _start_future to None in AsgiWorkerEntrypoint.__init__(). Could adjust it in a followup if you like though.
8b7b84c to
afea9e1
Compare
|
So we do actually want it set on the class because the instance is re-created on every worker invocation. The lifespan is supposed to stay alive for the duration of the lifetime of the isolate. I added a comment to explain this. |
That is really weird, can we migrate away from that? |
|
In JavaScript is the WorkerEntrypoint instantiated on every request? |
|
Yes, according to https://developers.cloudflare.com/workers/runtime-apis/bindings/service-bindings/rpc/#the-workerentrypoint-class
|
afea9e1 to
9beb25b
Compare
|
Right it also says:
What is this persistent lifespan state being used for? Is it volatile cache kind of stuff? Would be interested if you could post the use case. |
|
As far as I understand it's for reconstructible event-loop-local resources like HTTP clients, database pools, etc. Perhaps it isn't all that useful for Workers, but existing FastAPI apps that wish to run on Workers may find it useful. It's just about keeping things that are reused by each requested handled inside a single isolate. If you don't think it's worth the complexity that's fine too. We can close :) |
Ensures that mutable lifespans are persisted across requests. Adds fastapi and asgi-specific tests.